Skip to content

[wasm-split] Pre-create trampolines for global initializers#8542

Open
aheejin wants to merge 4 commits intoWebAssembly:mainfrom
aheejin:wasm_split_fuzz_fix
Open

[wasm-split] Pre-create trampolines for global initializers#8542
aheejin wants to merge 4 commits intoWebAssembly:mainfrom
aheejin:wasm_split_fuzz_fix

Conversation

@aheejin
Copy link
Copy Markdown
Member

@aheejin aheejin commented Mar 29, 2026

Before #8443, we scanned ref.funcs in global initializers early in indirectReferencesToSecondaryFunctions and created trampolines for them and replaced ref.func $funcs with ref.func $trampoline_func if func was set to move to a secondary module. But in case the global containing ref.func $trampoline_func also ends up moving to the same secondary module, creating trampoline and using it was not necessary, because the global can simply use ref.func $func because func is in the same secondary module.

To fix this, in #8443, we postponed creating trampolines for ref.funcs in global initializers until shareImportableItems. This had a problem, because we end up creating new trampolines late in shareImportableItems. But trampolines were designed to go through indirectCallsToSecondaryFunctions and setupTablePatching, so those late trampolines were invalid, like

(func $trampoline_foo
  (call $foo)
)

when foo was in a secondary module. This was supposed to be converted to a call_indirect in indirectCallsToSecondaryFunctions and the table elements were supposed to set up in setupTablePatching.


To fix the issue, this PR pre-creates trampolines for ref.funcs in all global initializers in indirectReferencesToSecondaryFunctions, before indirectCallsToSecondaryFunctions and setupTablePatching, but doesn't make ref.funcs reference them just yet. We make ref.funcs in global initializers reference the trampolines only when it is decided they are not moving to the same secondary module, in shareImportableItems.

But this can leave unused trampolines in the primary module. So in cleanupUnusedTrampolines, we remove unused trampolines.

This increases acx_gallery primary module size only by 0.6%. If we didn't clean up the unnecessary trampolines, the increase was 3.6%. This only removes unused trampoline functions and not placeholder imports and elements yet, but they don't affect acx_gallery because it uses --no-placeholders. The running time hasn't meaningfully changed.

Fixes #8510. (Both tests fail for the same reason)

Before WebAssembly#8443, we scanned `ref.func`s in global initizliers early in
`indirectReferencesToSecondaryFunctions` and created trampolines for
them and replaced `ref.func $func`s with `ref.func $trampoline_func` if
`func` was set to move to a secondary module. But in case the global
containing `ref.func $trampoline_func` also ends up moving to the same
secondary module, creating trampoline and using it was not necessary,
because the global can simply use `ref.func $func` because `func` is in
the same secondary module.

To fix this, in WebAssembly#8443, we postponed creating trampolines for `ref.func`s
in global initializers until `shareImportableItems`. This had a problem,
because we end up creating new trampolines late in
`shareImportableItems`. But trampolines were designed to go through
`indirectCallsToSecondaryFunctions` and `setupTablePatching`, so those
late trampolines were invalid, like
```wast
(func $trampoline_foo
  (call $foo)
)
```
when `foo` is in a secondary module. This was supposed to be converted
to a `call_indirect` in `indirectCallsToSecondaryFunctions` and the
table elements were supposed to set up in `setupTablePatching`.

---

To fix the issue, this PR pre-creates trampolines for `ref.func`s in
all global initializers in `indirectReferencesToSecondaryFunctions`,
before `indirectCallsToSecondaryFunctions` and `setupTablePatching`, but
doesn't make `ref.func`s reference them just yet. We make `ref.func`s in
global initializers reference the trampolines only when it is decided
they are not moving to the same secondary module, in
`shareImportableItems`.

But this can leave unused trampolines in the primary module. So in
`cleanupUnusedTrampolines`, we remove unused trampolines.

This increases acx_gallery code size only by 0.6%. If we didn't clean up
the unnecessary trampolines, the increase was 3.6%. This only removes
unused trampoline functions and not placeholder imports and elements
yet, but they don't affect acx_gallery because it uses
`--no-placeholders`.

Fixes WebAssembly#8510. (Both tests fail for the same reason)
@aheejin aheejin requested a review from tlively March 29, 2026 11:13
@aheejin aheejin requested a review from a team as a code owner March 29, 2026 11:13
@@ -0,0 +1,51 @@
;; RUN: wasm-split %s -all -g -o1 %t.1.wasm -o2 %t.2.wasm --keep-funcs=keep
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file was renamed from global-funcref.wast‎, and the expectations were rewritten. Previously I tried to put each line of expectation above the corresponding code line, but it's not easy when we have many other things like imports, exports, and trampolines.

Comment on lines +512 to +514
// Generate the call and the function. We generate a direct call here, but
// this will be converted to a call_indirect in
// indirectCallsToSecondaryFunctions.
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Drive-by comment improvement. Not related to this PR

;; SECONDARY: (import "primary" "prime" (func $prime (exact (type $0))))

;; SECONDARY: (elem $0 (i32.const 0) $second-in-table $second)
;; SECONDARY: (elem $0 (i32.const 0) $second $second-in-table)
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This depends on the trampoline creating order. Not meaningful

@aheejin aheejin removed the request for review from a team March 29, 2026 11:22
exportImportCalledPrimaryFunctions();
setupTablePatching();
shareImportableItems();
cleanupUnusedTrampolines();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be good to update the big comment at the top of the file to include this new step.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done: 8501ed6

// primary module, it means it was created for a global initializer but that
// global ended up moving to the same secondary module as the function
// referenced in the global initializer's ref.func. We can remove them here.
// TODO Remove placeholders and table elements created by unused trampolines
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fix conservatively creates extra trampolines, but then we have to clean them up afterwards. What would it look like to fix the bug by instead moving the globals to secondary modules earlier or being more precise about which globals need to use trampolines in their initializers? That way we wouldn't have to clean up afterwards.

Copy link
Copy Markdown
Member Author

@aheejin aheejin Apr 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the long delay!

I think this is doable, but this basically means we split shareImportableItems into two methods, one for globals and the other for tables/memorys/tags, or, one for global/memory/tags and the other for tables, because, we can't move table analysis before setupTablePatching. I think the current way can be a cleaner compromise than having two split functions of shareImporTableItems. WDYT? If you feel strongly with the splitting globals first, I can try.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's ok to handle globals separately from the rest of the items handled in shareImportableItems. We're now moving globals into different modules, which makes them more similar to functions than they are to the rest of the items here. My intuition is that moving globals to secondary modules right after we move functions to secondary modules and before we start creating any trampolines at all will be the simplest solution because we wouldn't have to treat globals or their initializers any differently from other module-level code after that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

wasm-split fuzz bug

2 participants